Open
Conversation
AdamGS
force-pushed
the
adamg/metrics-v2
branch
from
e28c2c5 to
5a80e52
Compare
AdamGS
force-pushed
the
adamg/metrics-v2
branch
from
5a80e52 to
2b519f2
Compare
AdamGS
commented
Jan 28, 2026
| @@ -0,0 +1 @@ | |||
| Cargo.lock No newline at end of file | |||
Contributor
Author
There was a problem hiding this comment.
this is just some housekeeping I ran into
AdamGS
force-pushed
the
adamg/metrics-v2
branch
from
7c00333 to
cccc3d5
Compare
Merging this PR will degrade performance by 33.09%
Performance Changes
Comparing 
Footnotes
|
AdamGS
force-pushed
the
adamg/metrics-v2
branch
2 times, most recently
from
215fcde to
ab32105
Compare
AdamGS
added
action/benchmark-sql
github-actions
bot
removed
the
action/benchmark-sql
Contributor
🚨🚨🚨❌❌❌ SQL BENCHMARK FAILED ❌❌❌🚨🚨🚨Benchmark |
Contributor
🚨🚨🚨❌❌❌ SQL BENCHMARK FAILED ❌❌❌🚨🚨🚨Benchmark |
Contributor
Benchmarks: TPC-H SF=1 on NVMESummary
Detailed Results Table
|
Contributor
Benchmarks: FineWeb NVMeSummary
Detailed Results Table
|
Contributor
Benchmarks: TPC-DS SF=1 on NVMESummary
Detailed Results Table
|
Contributor
Benchmarks: FineWeb S3Summary
Detailed Results Table
|
Contributor
Benchmarks: TPC-H SF=10 on NVMESummary
Detailed Results Table
|
Contributor
Benchmarks: TPC-H SF=10 on S3Summary
Detailed Results Table
|
Contributor
Benchmarks: TPC-H SF=1 on S3Summary
Detailed Results Table
|
AdamGS
force-pushed
the
adamg/metrics-v2
branch
from
ab32105 to
32d8be0
Compare
github-actions
bot
removed
the
action/benchmark-sql
Contributor
Benchmarks: Statistical and Population GeneticsSummary
Detailed Results Table
|
Contributor
Benchmarks: Clickbench on NVMESummary
Detailed Results Table
|
AdamGS
force-pushed
the
adamg/metrics-v2
branch
from
32d8be0 to
7dfab2b
Compare
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
AdamGS
force-pushed
the
adamg/metrics-v2
branch
from
dcc1c02 to
a56d1ea
Compare
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
AdamGS
force-pushed
the
adamg/metrics-v2
branch
from
a56d1ea to
cfff979
Compare
2 tasks
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
AdamGS
added a commit
that referenced
this pull request
Feb 3, 2026
The tests are failing due to a bug in the `url` crate, I had a fix as part of #6187 but it's taking longer than expected to get it in, so figured I'll just take it out. Signed-off-by: Adam Gutglick <adam@spiraldb.com>
robert3005
reviewed
Feb 3, 2026
| type Item = (MetricId, &'a Metric); | ||
| /// Default metrics registry, stores all state in-memory. | ||
| #[derive(Default, Clone)] | ||
| pub struct DefaultMetricsRegistry { |
Contributor
There was a problem hiding this comment.
Debating whether we want a RwLock or Mutex here
robert3005
reviewed
Feb 3, 2026
Contributor
robert3005
left a comment
robert3005
left a comment
There was a problem hiding this comment.
I made couple small comments. The thing that I am not convinced by is double arcing. But I can see how it ends up being necessary
Comment on lines
+4
to
+5
| #![allow(clippy::expect_used)] | ||
| #![allow(clippy::unwrap_in_result)] |
Comment on lines
+4
to
+5
| #![allow(clippy::expect_used)] | ||
| #![allow(clippy::unwrap_in_result)] |
|
|
||
| /// Sets the gauge to a specific value. | ||
| pub fn set(&self, value: f64) { | ||
| _ = self.0.swap(value.to_bits(), Ordering::AcqRel); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #6078.
This PR reworks our metrics crate to simplify it and makes it much harder to make it use unbounded amounts of memory. The implementation is inspired by our current use of
witchcraft-metrics, themetricscrate, DataFusion's metrics and some reading of the OTel spec.It includes several changes:
VortexMetricsis replaced by the newMetricsRegistrytrait, which allows user to integrate metrics to whatever metrics backend they have.witchcraft-metricsdependency. It does many things we don't care about or that are opinionated in a way that I don't think is intuitive or useful for most users. It also makes our external API less dependent on other crates we have no control over.Duration, and counters are u64s. It also introduces aGaugetype which I don't think we actually use and I might just delete.I've tested it by running our DF benchmarks with
--show-metricsand that seems to not change, if anyone else has made use of the metrics, I would love to help test and debug them in another context.